Bound and test Reflex compatibility window - #475
Conversation
📝 WalkthroughWalkthroughThe pull request bounds the ChangesReflex compatibility window
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant WorkflowValidator
participant PackageMetadata
participant CompatibilitySmokes
CIWorkflow->>WorkflowValidator: validate reflex_compatibility matrix
WorkflowValidator->>PackageMetadata: check xy[reflex] requirement
CIWorkflow->>CompatibilitySmokes: run component and state tests on 0.9.6 and 0.9.8
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify_ci_workflow.py (1)
1153-1162: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the bounded requirement in both coreless sdist smoke steps.
The coreless CI and release checks do not require
REFLEX_REQUIREMENT. A future change can remove the upper bound from either coreless installation command without failingscripts/verify_ci_workflow.py.
scripts/verify_ci_workflow.py#L1153-L1162: requiref'"{REFLEX_REQUIREMENT}"'in the CI coreless-sdist step.scripts/verify_ci_workflow.py#L1364-L1373: requiref'"{REFLEX_REQUIREMENT}"'in the release coreless-sdist step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify_ci_workflow.py` around lines 1153 - 1162, The coreless sdist smoke-step validation must enforce the bounded REFLEX_REQUIREMENT in both locations. Update _require_step_contains at scripts/verify_ci_workflow.py lines 1153-1162 and 1364-1373 to require f'"{REFLEX_REQUIREMENT}"' for the CI and release coreless-sdist steps, respectively.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
251-253: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable persisted checkout credentials.
actions/checkoutstores the workflow token in local Git configuration by default. The laternpm ci, package build, and test commands can read that credential. Setpersist-credentials: false. No displayed step needs Git write access.Proposed change
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 251 - 253, Update the actions/checkout step to set persist-credentials to false alongside fetch-depth, ensuring the checkout token is not stored in local Git configuration.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 888-897: Update the project metadata validation around
REFLEX_REQUIREMENT to parse pyproject.toml as TOML and inspect
project.optional-dependencies.reflex directly. Require that the reflex extra
exists and contains exactly REFLEX_REQUIREMENT, rejecting placements in other
dependency groups, base dependencies, comments, or additional reflex
requirements while preserving the existing read-error reporting.
---
Outside diff comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 1153-1162: The coreless sdist smoke-step validation must enforce
the bounded REFLEX_REQUIREMENT in both locations. Update _require_step_contains
at scripts/verify_ci_workflow.py lines 1153-1162 and 1364-1373 to require
f'"{REFLEX_REQUIREMENT}"' for the CI and release coreless-sdist steps,
respectively.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 251-253: Update the actions/checkout step to set
persist-credentials to false alongside fetch-depth, ensuring the checkout token
is not stored in local Git configuration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 317d8e05-a228-443d-ae92-131597dd25f3
📒 Files selected for processing (6)
.github/workflows/ci.yml.github/workflows/release.ymldocs/advanced/runtime-and-deployment.mdpyproject.tomlscripts/verify_ci_workflow.pyspec/process/production-readiness.md
| try: | ||
| project_metadata = (ROOT / "pyproject.toml").read_text(encoding="utf-8") | ||
| except OSError as exc: | ||
| errors.append(f"cannot read project metadata for Reflex compatibility gate: {exc}") | ||
| else: | ||
| if f'"{REFLEX_REQUIREMENT}"' not in project_metadata: | ||
| errors.append( | ||
| "pyproject.toml must bound the xy[reflex] extra to " | ||
| f"{REFLEX_REQUIREMENT!r}" | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the reflex extra structurally.
Line 893 only searches for the requirement string in the full TOML text. It passes if the requirement moves to another extra, a base dependency, or a comment. Plain xy could then install Reflex, or xy[reflex] could omit it.
Parse pyproject.toml and require project.optional-dependencies.reflex to contain exactly REFLEX_REQUIREMENT.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/verify_ci_workflow.py` around lines 888 - 897, Update the project
metadata validation around REFLEX_REQUIREMENT to parse pyproject.toml as TOML
and inspect project.optional-dependencies.reflex directly. Require that the
reflex extra exists and contains exactly REFLEX_REQUIREMENT, rejecting
placements in other dependency groups, base dependencies, comments, or
additional reflex requirements while preserving the existing read-error
reporting.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/verify_ci_workflow.py">
<violation number="1" location="scripts/verify_ci_workflow.py:885">
P2: The new Reflex compatibility gate can pass with no active Reflex install or smoke tests because it searches the raw job text, including comments and inactive YAML, rather than executable named steps. Structural checks of the install and smoke-test steps would keep CI from silently losing this coverage.</violation>
<violation number="2" location="scripts/verify_ci_workflow.py:893">
P2: The metadata check does not prove that `xy[reflex]` is bounded: an unrelated occurrence of the exact requirement anywhere in `pyproject.toml` can make it pass. Parsing `project.optional-dependencies.reflex` and validating that entry directly would ensure the published extra, not a decoy string, stays within the tested window.</violation>
</file>
<file name="spec/process/production-readiness.md">
<violation number="1" location="spec/process/production-readiness.md:395">
P3: The new bounded Reflex contract (`reflex>=0.9.6,<0.10`) isn't propagated consistently in this file: the sdist release gate in the "Release-Blocking Gates" table (line ~92) still states the sdist `PKG-INFO` must include `reflex>=0.9.6` with no upper bound, contradicting the window introduced here. The confirming gate code also still fullmatches the unbounded form: `scripts/artifact_metadata.py` `_is_exact_reflex_extra` uses `re.fullmatch(r"\s*reflex\s*>=\s*0\.9\.6\s*;\s*extra...")` with no `,<0.10`, so it would reject the now-documented bounded `Requires-Dist: reflex>=0.9.6,<0.10; extra == "reflex"`. Update the sdist gate description (and reconcile the verifying script) so the documented window and its enforcement agree.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| except OSError as exc: | ||
| errors.append(f"cannot read project metadata for Reflex compatibility gate: {exc}") | ||
| else: | ||
| if f'"{REFLEX_REQUIREMENT}"' not in project_metadata: |
There was a problem hiding this comment.
P2: The metadata check does not prove that xy[reflex] is bounded: an unrelated occurrence of the exact requirement anywhere in pyproject.toml can make it pass. Parsing project.optional-dependencies.reflex and validating that entry directly would ensure the published extra, not a decoy string, stays within the tested window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/verify_ci_workflow.py, line 893:
<comment>The metadata check does not prove that `xy[reflex]` is bounded: an unrelated occurrence of the exact requirement anywhere in `pyproject.toml` can make it pass. Parsing `project.optional-dependencies.reflex` and validating that entry directly would ensure the published extra, not a decoy string, stays within the tested window.</comment>
<file context>
@@ -870,6 +872,29 @@ def validate_ci_workflow(path: Path = DEFAULT_CI_WORKFLOW) -> list[str]:
+ except OSError as exc:
+ errors.append(f"cannot read project metadata for Reflex compatibility gate: {exc}")
+ else:
+ if f'"{REFLEX_REQUIREMENT}"' not in project_metadata:
+ errors.append(
+ "pyproject.toml must bound the xy[reflex] extra to "
</file context>
| 'reflex-version: ["0.9.6", "0.9.8"]', | ||
| 'reflex==${{ matrix.reflex-version }}', | ||
| "<0.10,>=0.9.6", | ||
| "test_component.py::test_component_compiles_with_events", |
There was a problem hiding this comment.
P2: The new Reflex compatibility gate can pass with no active Reflex install or smoke tests because it searches the raw job text, including comments and inactive YAML, rather than executable named steps. Structural checks of the install and smoke-test steps would keep CI from silently losing this coverage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/verify_ci_workflow.py, line 885:
<comment>The new Reflex compatibility gate can pass with no active Reflex install or smoke tests because it searches the raw job text, including comments and inactive YAML, rather than executable named steps. Structural checks of the install and smoke-test steps would keep CI from silently losing this coverage.</comment>
<file context>
@@ -870,6 +872,29 @@ def validate_ci_workflow(path: Path = DEFAULT_CI_WORKFLOW) -> list[str]:
+ 'reflex-version: ["0.9.6", "0.9.8"]',
+ 'reflex==${{ matrix.reflex-version }}',
+ "<0.10,>=0.9.6",
+ "test_component.py::test_component_compiles_with_events",
+ "test_state_bridge.py::test_rebuild_reads_session_state",
+ )
</file context>
| `METADATA` must keep `Name: xy`, `Requires-Python: >=3.11`, | ||
| `anywidget>=0.9`, and `numpy>=1.24` as base requirements, plus | ||
| `Provides-Extra: reflex` and `reflex>=0.9.6` guarded by that extra. The wheel | ||
| `Provides-Extra: reflex` and `reflex>=0.9.6,<0.10` guarded by that extra. The wheel |
There was a problem hiding this comment.
P3: The new bounded Reflex contract (reflex>=0.9.6,<0.10) isn't propagated consistently in this file: the sdist release gate in the "Release-Blocking Gates" table (line ~92) still states the sdist PKG-INFO must include reflex>=0.9.6 with no upper bound, contradicting the window introduced here. The confirming gate code also still fullmatches the unbounded form: scripts/artifact_metadata.py _is_exact_reflex_extra uses re.fullmatch(r"\s*reflex\s*>=\s*0\.9\.6\s*;\s*extra...") with no ,<0.10, so it would reject the now-documented bounded Requires-Dist: reflex>=0.9.6,<0.10; extra == "reflex". Update the sdist gate description (and reconcile the verifying script) so the documented window and its enforcement agree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At spec/process/production-readiness.md, line 395:
<comment>The new bounded Reflex contract (`reflex>=0.9.6,<0.10`) isn't propagated consistently in this file: the sdist release gate in the "Release-Blocking Gates" table (line ~92) still states the sdist `PKG-INFO` must include `reflex>=0.9.6` with no upper bound, contradicting the window introduced here. The confirming gate code also still fullmatches the unbounded form: `scripts/artifact_metadata.py` `_is_exact_reflex_extra` uses `re.fullmatch(r"\s*reflex\s*>=\s*0\.9\.6\s*;\s*extra...")` with no `,<0.10`, so it would reject the now-documented bounded `Requires-Dist: reflex>=0.9.6,<0.10; extra == "reflex"`. Update the sdist gate description (and reconcile the verifying script) so the documented window and its enforcement agree.</comment>
<file context>
@@ -392,7 +392,7 @@ Before tagging a release:
`METADATA` must keep `Name: xy`, `Requires-Python: >=3.11`,
`anywidget>=0.9`, and `numpy>=1.24` as base requirements, plus
- `Provides-Extra: reflex` and `reflex>=0.9.6` guarded by that extra. The wheel
+ `Provides-Extra: reflex` and `reflex>=0.9.6,<0.10` guarded by that extra. The wheel
must contain `reflex_xy` and `XYChart.jsx`, and `RECORD` must list every
archive file exactly once with matching `sha256` and size fields. Wheels
</file context>
1 participant
Fixes #447
Summary
Testing
Summary by CodeRabbit
Compatibility
Documentation
Quality Improvements